Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New pickit property skin #1421

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

New pickit property skin #1421

wants to merge 1 commit into from

Conversation

ryancrunchi
Copy link
Contributor

@ryancrunchi ryancrunchi commented Apr 5, 2019 •
edited
Loading

  • Prototype.js, NTItemAlias.dbl, NTItemParser.dbl

    • Added new property keyword Skin for pickit to filter rings, amulets, jewels and charms skin.

  • GameAction.js, MuleLogger.js, Misc.js

    • Refactored getting the skin value of item with property skinCode in Unit prototype.

Possible skin values are based on these :
image
image

  • Rings :

    • coral or chain
    • smallblue or sloop
    • bigblue or bband
    • orangestone or orangering
    • crown or eturn

  • Amulets :

    • dot or cross
    • sun
    • penta or star

  • Jewels :

    • pink
    • blue
    • orange or peach
    • green
    • red
    • white

  • Small charms :

    • brown or football
    • bear or bearfoot or bearclaw
    • m or coin

  • Large charms :

    • paw or paper
    • horn
    • tower or obelisk

  • Grand charms :

    • eye
    • dna or lace or spaghetti
    • dragon or monster

  • Usage :
    You can specify the skin without item to be identified. This means that the skin property goes the same way as type, name or quality.
    In your pickit :

// keep any unique rings unid with coral skin
[type] == ring && [quality] == unique && [skin] == coral
// or even shorter, as the skin is related to the item type. Only rings can have coral skin.
[quality] == unique && [skin] == coral

// keep any mara with dot skin
[type] == amulet && [quality] == unique && [skin] == dot # [strength] == 5 && [ColdResist] >= 20

// keep perf light rbf white
[type] == jewel && [quality] == unique && [skin] == white # [PassiveLtngMastery]+[PassiveLtngPierce] == 10
  • Caution :
    If by mistake you specify a skin that does not exist for the item type, this item will NOT be kept.
    For example
    [type] == ring && [quality] == unique && [skin] == orange # [ItemAllSkills] > 0
    It will NOT take any soj or bk because orange skin does not exist for rings.

@imbalanced
Copy link
Contributor

image

This is the common names I've seen being used. You may want to refer to this for alias as well.

imbalanced
imbalanced reviewed Apr 6, 2019
View reviewed changes
d2bs/kolbot/libs/NTItemAlias.dbl Outdated
// small charms
NTIPAliasSkin["brown"] = "cm11";
NTIPAliasSkin["bear"] = "cm12"; NTIPAliasSkin["bearfoot"] = "cm12";
NTIPAliasSkin["m"] = "cm13"; NTIPAliasSkin["M"] = "cm13";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalized aliases are unnecessary as they are lower cased in the parser anyways.

imbalanced
imbalanced reviewed Apr 6, 2019
View reviewed changes
d2bs/kolbot/libs/NTItemAlias.dbl Outdated
NTIPAliasSkin["tower"] = "cm23";
// grand charms
NTIPAliasSkin["eye"] = "cm31";
NTIPAliasSkin["dna"] = "cm32"; NTIPAliasSkin["DNA"] = "cm32"; NTIPAliasSkin["spaghetti"] = "cm32";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Capitalized aliases are unnecessary as they are lower cased in the parser anyways.

@noah-
Copy link
Collaborator

noah- commented Apr 6, 2019

can you change skinCode to getSkinCode()
it should not be a value that can be set

@ryancrunchi
Copy link
Contributor Author

Defining a property with a get only function avoid this property to be writable.
If you try to do so, you will get an error like Uncaught TypeError: Cannot set property skinCode of #<Unit> which has only a getter.

@noah-
Copy link
Collaborator

noah- commented Apr 7, 2019

Sure, but that is at runtime - changing it to a get function makes it’s clear statically that it is not a property of the object, and follows convention with getColor.

noah-
noah- requested changes Apr 8, 2019
View reviewed changes
d2bs/kolbot/libs/common/Prototypes.js
@@ -1039,6 +1039,158 @@ Unit.prototype.getColor = function () {
return -1;
};

Object.defineProperty(Unit.prototype, "skinCode", {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to getSkinCode() to match existing kolbot convention (eg getColor)

@noah- noah- self-requested a review April 13, 2019 23:12
@jaenster
Copy link
Contributor

jaenster commented Jun 8, 2019 •
edited
Loading

Sure, but that is at runtime - changing it to a get function makes it’s clear statically that it is not a property of the object, and follows convention with getColor.

I understand what your saying, but there are loads of these kinds of "calculated" values in kolton and d2bs already.

Stuff that isnt settable, but retrievable as an variable on the unit object, like: idle, gold,dead,inTown,attacking,parentName, dexreq, strreq and itemclass. And those build-in d2bs variables on units, like classid, type, mode, name, act gid, x ,y, hp , hpmax, mp ,mpmax , stamina, staminamax, charlvl, itemcount, owner, ownertype, uniqueid, code, prefixes,suffixes,prefixnum,suffixnum, fname, quality, node, locations, sizex,sizey, itemtype, description, bodylocation, ilvl, lvlreq and gfx

Those are all already on the item unit, why can't skinCode be one of them?

skinCode seems to make sense to be also one of these variables.

@ryancrunchi
Added new property keyword Skin for pickit to filter rings, amulets…
6dfa44b 
…, jewels and charms skin.

Refactored getting the skin value of item with property `skinCode` in Unit prototype.
@noah-
Copy link
Collaborator

noah- commented Jun 16, 2019 •
edited
Loading

Sure, but that is at runtime - changing it to a get function makes it’s clear statically that it is not a property of the object, and follows convention with getColor.

I understand what your saying, but there are loads of these kinds of "calculated" values in kolton and d2bs already.

Stuff that isnt settable, but retrievable as an variable on the unit object, like: idle, gold,dead,inTown,attacking,parentName, dexreq, strreq and itemclass. And those build-in d2bs variables on units, like classid, type, mode, name, act gid, x ,y, hp , hpmax, mp ,mpmax , stamina, staminamax, charlvl, itemcount, owner, ownertype, uniqueid, code, prefixes,suffixes,prefixnum,suffixnum, fname, quality, node, locations, sizex,sizey, itemtype, description, bodylocation, ilvl, lvlreq and gfx

Those are all already on the item unit, why can't skinCode be one of them?

skinCode seems to make sense to be also one of these variables.

The main difference is those are done by d2bs or are extremely trivial, this is a change in kolbot so it should follow kolbot convention for similar functions (like getColor).

@ryancrunchi ryancrunchi mentioned this pull request Nov 3, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imbalanced imbalanced imbalanced left review comments

@noah- noah- Awaiting requested review from noah-

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ryancrunchi @imbalanced @noah- @jaenster